Convert shell script to Go #3413

Kalaiselvi84 · 2023-10-04T23:07:58Z

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

build/export-openapi.sh

agones-bot · 2023-10-04T23:31:21Z

Build Failed 😱

Build Id: 4a6b92de-88d8-45e1-babc-ff78d326c5f7

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-10-05T17:11:35Z

Build Succeeded 👏

Build Id: cb88331b-8784-4b36-ab67-8834f183ef0c

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://236ba26-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-236ba26-amd64

agones-bot · 2023-10-05T18:49:48Z

Build Succeeded 👏

Build Id: add2ca0e-d222-477a-8f03-015fc6436d13

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://4192499-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-4192499-amd64

markmandel · 2023-10-05T19:10:08Z

@gongmax I'll let you decide when we've got what we need from this 👍🏻

agones-bot · 2023-10-06T21:58:37Z

Build Succeeded 👏

Build Id: d6a83ed9-30ad-45db-9590-68c9aa8a1726

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://78e6ec5-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-78e6ec5-amd64

build/export-openapi.sh

markmandel · 2023-10-11T17:20:59Z

Meta question: Is it time to convert this into a little Go program?

build/replaceRef.go

agones-bot · 2023-10-11T21:58:01Z

Build Succeeded 👏

Build Id: 1d354c62-854f-4b55-a0fc-e8393518eccb

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://e17aa7d-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-e17aa7d-amd64

Kalaiselvi84 · 2023-10-12T16:46:20Z

@markmandel, I really need your help to convert the bash script into a Go script. It looks like the initial steps in the bash script to set the shell options (errexit, nounset, and pipefail) aren't required in Go. Please correct me if I'm wrong. Could you please kindly split the tasks and share the steps?

install/helm/agones/templates/crds/k8s/_io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta.yaml

agones-bot · 2023-10-19T23:45:03Z

Build Failed 😱

Build Id: 0f496cc1-c315-4e85-85a3-0a4e47f67473

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2023-10-19T23:58:56Z

make[1]: Leaving directory '/workspace/build'
sort /workspace/install/yaml/install.yaml > /tmp/agones-install/install.current.yaml.sorted
diff /tmp/agones-install/install.yaml.sorted /tmp/agones-install/install.current.yaml.sorted
101a102,104

Will also need a make gen-install 😄 I forgot that step.

markmandel · 2023-10-20T00:01:29Z

build/scripts/k8s-export-openapi/main.go

+	}
+
+	// Start the kubectl proxy.
+	cmd := exec.Command("kubectl", "proxy")


So we're running kubectl proxy here... so it should be started.

We might want to print/log the output from the command, to make sure it's doing what we expect.

(this might need to be put in a Go routine, since calling any of the output commands such as: https://pkg.go.dev/os/exec#Cmd.Output will block).

Can he be implemented using client-go here?

Goroutine encountered an error: Failed to start kubectl proxy: signal: interrupt. I've excluded it.

I used the same target, gen-embedded-openapi, and it successfully executed using the Go script with the -mod=mod option. No modifications were needed in the Kubernetes checklist.

Pending task:
Check nullable=true missing in yaml file

build/scripts/k8s-export-openapi/main.go

agones-bot · 2023-10-20T01:48:30Z

Build Failed 😱

Build Id: 3107b9d9-3295-4f51-bb75-daa2f2d09503

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-10-20T03:19:49Z

Build Failed 😱

Build Id: 3d1fc787-038f-481d-b272-de3cb32f6a90

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-10-20T16:23:28Z

Build Failed 😱

Build Id: 7fa293a3-0c19-41a5-8b20-6d8e6d54031e

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2023-10-20T16:23:53Z

go.mod

@@ -74,6 +74,7 @@ require (
 	github.com/googleapis/gax-go/v2 v2.12.0 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/imdario/mergo v0.3.6 // indirect
+	github.com/itchyny/json2yaml v0.1.4 // indirect


This should all be reset - there should be changes to the go.mod here or the go.sum I don't think.

markmandel · 2023-10-20T16:24:05Z

vendor/modules.txt

@@ -239,6 +239,8 @@ github.com/heptiolabs/healthcheck
 # github.com/imdario/mergo v0.3.6
 ## explicit
 github.com/imdario/mergo
+# github.com/itchyny/json2yaml v0.1.4


I don't think this should be here either.

CI build complains🤔

go: inconsistent vendoring in /go/src/agones.dev/agones:
github.com/itchyny/json2yaml@v0.1.4: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt

To ignore the vendor directory, use -mod=readonly or -mod=mod. To sync the vendor directory, run: go mod vendor

Alright, if you say so, there must be a reason behind it. I've reverted the changes in go.sum, go.mod, and vendor/modules.txt

Yaml files look good to me. Could you please review the updated yaml files and share your thoughts?

Oh no, If I run the code the go.sum and go.mod files are automatically updated. I guess this is because of -mod=mod?

gen-embedded-openapi: ensure-build-image docker run --rm $(common_mounts) --workdir=$(mount_path) $(DOCKER_RUN_ARGS) $(build_tag) \ go run -mod=mod build/scripts/k8s-export-openapi/main.go

However, the code is failing without -mod=mod. What could be the solution?

Maybe a silly question - what happens if we do:

gen-embedded-openapi: ensure-build-image docker run --rm $(common_mounts) --workdir=$(mount_path)/build/scripts/k8s-export-openapi $(DOCKER_RUN_ARGS) $(build_tag) \ go run -mod=mod ./main.go

(Might not need the -mod=mod either at that point?)

I believe that if the workdir changes, the paths to other files in the code will also need to be modified.

Try it? I expect the script will fail, but we'll see if it updates the other go.mod and go.sum files -- and if it doesn't, then we know making the path changes worth it.

The good news is - with the agones/go.mod and agones/go.sum changes reverted - CI is passing again - so that's at least a step in the right direction!

Script passed with the below modifications and it didn't modify the go.mod and go.sum files.

tmpDir := "../../tmp"
boilerplateContent, err := os.ReadFile("../../boilerplate.yaml.txt")
outputPath := filepath.Join("../../../install/helm/agones/templates/crds/k8s", "_"+filename+".yaml")

Love it 😄

agones-bot · 2023-10-20T18:21:55Z

Build Succeeded 👏

Build Id: e96e0a2a-da2f-41ff-8f48-4e392e5710d7

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://7f12569-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-7f12569-amd64

agones-bot · 2023-10-20T19:38:17Z

Build Succeeded 👏

Build Id: 5a0fbf76-e380-435f-832d-822bce7d3456

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://720452e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3413/head:pr_3413 && git checkout pr_3413
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-720452e-amd64

Kalaiselvi84 · 2023-10-20T19:44:52Z

CI build came out green. Does everything look good?

markmandel

This looks good to me! @gongmax did you want to also give it a once over before I merge, to make sure I didn't miss anything?

gongmax · 2023-10-20T20:03:35Z

This looks good to me! @gongmax did you want to also give it a once over before I merge, to make sure I didn't miss anything?

Taking a look...

google-oss-prow · 2023-10-20T20:10:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gongmax, Kalaiselvi84, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markmandel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

markmandel · 2023-10-20T20:11:10Z

YAY!

THE BASH MONSTROSITY I SHOULD NEVER HAVE WRITTEN IN THE FIRST PLACE IS NOW GONE!

Change to awk

236ba26

Kalaiselvi84 requested a review from gongmax October 4, 2023 23:07

google-oss-prow bot added the size/XS label Oct 4, 2023

github-actions bot added the kind/cleanup Refactoring code, fixing up documentation, etc label Oct 4, 2023

google-oss-prow bot requested review from aLekSer and EricFortin October 4, 2023 23:08

gongmax requested review from markmandel and removed request for EricFortin and aLekSer October 4, 2023 23:21

markmandel reviewed Oct 4, 2023

View reviewed changes

build/export-openapi.sh Outdated Show resolved Hide resolved

excluded print and -F

4192499

Kalaiselvi84 added 2 commits October 6, 2023 20:56

xargs variation

59584f8

Merge branch 'main' into sed-to-awk

78e6ec5

gongmax reviewed Oct 9, 2023

View reviewed changes

build/export-openapi.sh Outdated Show resolved Hide resolved

go-script to replace ref

e17aa7d

google-oss-prow bot added size/M and removed size/XS labels Oct 11, 2023

Kalaiselvi84 changed the title ~~Use awk command~~ Fix export-openapi.sh file Oct 11, 2023

markmandel reviewed Oct 11, 2023

View reviewed changes

build/replaceRef.go Outdated Show resolved Hide resolved

gongmax reviewed Oct 11, 2023

View reviewed changes

build/replaceRef.go Outdated Show resolved Hide resolved

Kalaiselvi84 marked this pull request as draft October 11, 2023 23:21

google-oss-prow bot added the do-not-merge/work-in-progress label Oct 11, 2023

markmandel reviewed Oct 19, 2023

View reviewed changes

install/helm/agones/templates/crds/k8s/_io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta.yaml Show resolved Hide resolved

markmandel reviewed Oct 20, 2023

View reviewed changes

kubectl proxy - log, sleep time, graceful termination

98c6e0c

gen-embedded-openapi target for go script, run make gen-install

5f9f4db

Kalaiselvi84 added 2 commits October 20, 2023 15:54

agones - go mod vendor

ec09c3d

tmp dir path change

5e7ea72

markmandel reviewed Oct 20, 2023

View reviewed changes

Kalaiselvi84 added 2 commits October 20, 2023 16:39

fix modifyJSONFiles(), make gen-install

597745d

revert go.mod, go.sum and vendor/modules.txt

7f12569

Kalaiselvi84 added 2 commits October 20, 2023 18:33

change path

cebc3e2

Merge branch 'main' into sed-to-awk

720452e

markmandel approved these changes Oct 20, 2023

View reviewed changes

google-oss-prow bot assigned markmandel Oct 20, 2023

google-oss-prow bot added lgtm approved labels Oct 20, 2023

gongmax approved these changes Oct 20, 2023

View reviewed changes

google-oss-prow bot assigned gongmax Oct 20, 2023

gongmax merged commit 848ede9 into googleforgames:main Oct 20, 2023
3 checks passed

Kalaiselvi84 deleted the sed-to-awk branch March 15, 2024 01:11

Convert shell script to Go #3413

Convert shell script to Go #3413

Conversation

Kalaiselvi84 commented Oct 4, 2023

agones-bot commented Oct 4, 2023

agones-bot commented Oct 5, 2023

agones-bot commented Oct 5, 2023

markmandel commented Oct 5, 2023

agones-bot commented Oct 6, 2023

markmandel commented Oct 11, 2023

agones-bot commented Oct 11, 2023

Kalaiselvi84 commented Oct 12, 2023

agones-bot commented Oct 19, 2023

markmandel commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Oct 20, 2023

agones-bot commented Oct 20, 2023

agones-bot commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kalaiselvi84 Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kalaiselvi84 Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Oct 20, 2023

agones-bot commented Oct 20, 2023

Kalaiselvi84 commented Oct 20, 2023

markmandel left a comment

Choose a reason for hiding this comment

gongmax commented Oct 20, 2023

google-oss-prow bot commented Oct 20, 2023

markmandel commented Oct 20, 2023

Kalaiselvi84 Oct 20, 2023 •

edited

Loading

Kalaiselvi84 Oct 20, 2023 •

edited

Loading